fix(advisor): every association count names its population (FM5) - #17
Conversation
`learned_associations()` computed `observations` as the sum over `by_skill`
ALONE, so an advisory match recorded WITHOUT a skill was invisible in the
count while still being counted in `by_task_type`. The live ledger read
"observations: 3" beside 12 task-type observations, and the gap looked like a
ledger bug rather than a denominator bug.
That is a convenient denominator sitting in the reporting path of the very
tool meant to measure whether the skills are wired to the front door. Measured
with that number, "the skills are wired now" is unfalsifiable — the count can
only ever move for the population it already covers.
Three counts that each name their population and reconcile by construction:
observations every capability_advisor match event
observations_with_skill the subset naming a skill — the ONLY population
by_skill can cover
observations_without_skill counted in by_task_type, absent from by_skill
plus a `populations` map so a subset cannot pass as the set. On the live
ledger this reads 20 / 11 / 9 where the old code reported a bare "11".
Selftest: a skill-less advisory match must be visible in the totals and absent
from by_skill, and `observations > observations_with_skill` must hold — the old
definition made those two equal. Deliberate break -> revert performed:
restoring `"observations": with_skill` fails the new assertion and prints the
bug in the failure message (observations 2 beside by_task_type testgen 3, the
live symptom in miniature); the revert is byte-identical.
No behaviour change to classification, matching, recording or dispatch. No new
store, no second advisor, no lifecycle record touched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 25 minutes Limit details: You’ve used the included review currently available. Your 66 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
1 participant
Why
capability_advisor.learned_associations()— the reporting path of the tool that answers "are theskills actually wired to the front door?" — computed
observationsas the sum overby_skillalone. An advisory match recorded without a skill was therefore invisible in the count while
still being counted in
by_task_type.On the live ledger that reads:
The gap looks like a ledger bug. It is a denominator bug: FM5 (convenient denominators), sitting in
the reporting path of the very tool meant to measure the thing. Measured with that number, "the
skills are wired now" is unfalsifiable — the count can only ever move for the population it already
covers, so a rise proves nothing and a flat line hides everything.
What changed
capability_advisor.pyonly. Three counts that each name their population and reconcile byconstruction, plus a
populationsmap so a subset cannot pass as the set:observationscapability_advisormatch event in the ledgerobservations_with_skillby_skillcan coverobservations_without_skillby_task_type, absent fromby_skillwith_skill + without_skill == observationsis asserted. Live ledger now reads 20 / 11 / 9 wherethe old code reported a bare
11.No behaviour change to classification, matching, recording or dispatch. No new store, no second
advisor, no lifecycle record touched — this is the existing surface reporting honestly.
Test gate
Named assertions added to
capability_advisor._selftest(): an advisory call with no skill (whata session that forgets
skill=produces) must be absent fromby_skill, visible in the totals, andmust make
observations > observations_with_skillhold.Deliberate break → revert, performed: restoring
"observations": with_skillfails the newassertion and prints the bug in the failure message —
observations: 2besideby_task_type testgen: 3, the live symptom in miniature. The old definition made those two equal, sothe assertion cannot pass without the fix. Revert is byte-identical.
Verification
python3 verify.pygreen on this branch.python3 capability_advisor.py --selftestgreen;python3 capabilities.py --selftestand--json validategreen (41 rows,"valid": true, 0 errors).ORCH_STATE_DIRandORCH_LOCAL_RUNTIMEpointed at emptydirectories; the front-door section skips with the prerequisite named. No new skip, floor
unchanged.
Non-goals
TASK_SIGNALSor any capability matcher. Raising the hit rate so more things matchwould destroy the signal the counts exist to carry (design commitment Configure Renovate #1: the advisor must be able
to say no).
capability_advisoritself: it is an observer/reporting surface,and registering it would force a declared delivery outcome it cannot physically produce.
Context
Companion to a
~/.claude/skillschange (machine-local, not in this repo): seven skills now consultthe
capability_adviceMCP tool at task initiation with the user's real task text, takingby_skillfrom 1 distinct skill to 5. Hitting this denominator while measuring that is what surfaced the bug.
🤖 Generated with Claude Code